Conversation
kidboi666
requested review from
bootcampCodeit,
withyj-codeit and
Ahseo
and removed request for
bootcampCodeit and
withyj-codeit
There was a problem hiding this comment.
DS_Store는 Desktop Services Store 의 약자로, 애플에서 정의한 파일 포맷입니다.
애플의 맥OS X 시스템이 폴더에 접근할 때 생기며, 해당 폴더에 대한 메타데이터를 저장하는 파일입니다. 즉, 이 부분은 git에 따로 올라갈 필요가 없는 파일이에요! 추후 .gitignore에 대해서도 보실 수 있을텐데, https://www.toptal.com/developers/gitignore
이를 통해 git에 올라가지 않는 파일들을 한 번에 관리할 수 있으니 참고해주세요 :)
There was a problem hiding this comment.
깃헙 사용법에 익숙치 않아 엄한곳에서 시간을 쓰다보니 정작 이러한 부분을 놓쳤네요. 다음부턴 주의 하겠습니다!
There was a problem hiding this comment.
svg, png 다양한 이미지를 사용하신 것 같아요. 어떤 기준에서 나눠서 사용하셨는지 궁금해요!
일반적으로 svg는 깨지지는 않지만 계산이 요구되어서 단순한 이미지에 많이 사용되는데, 이에 맞게 잘 사용하신 것 같긴합니다.
- 안쓰는 svg파일들이 몇 개 존재하는 것 같아요. 추후 이런 부분들이 나중에 불필요한 용량 문제를 야기해서 사용자에게 웹사이트가 느리게 뜨는 경험들을 줄 수가 있으니 이러한 부분들은 삭제하시는게 좋습니다!
There was a problem hiding this comment.
이미지 파일 사용 기준은, 픽셀의 왜곡이나 손상이 있어선 안되는 회사 로고 같은 중요 부분들, 사용할 때 직접적으로 클릭하는 등의 상호작용이 필요한 버튼 등의 아이콘을 중점적으로 svg를 사용했고 나머지 조금 덜 중요하다 싶은 부분은 png 로 썻습니다!
더미 파일들은 변명의 여지가 없네요! 다음 주차때 꼼꼼히 확인해서 바로 반영해보겠습니다! ㅠ
| const $wrongMessage = document.getElementById('wrong_message'); // 비밀번호가 틀렸습니다 메세지 div | ||
| // const $signUpButton = document.querySelector('.signup_button'); | ||
|
|
||
| const valueWrong = (e) => { |
There was a problem hiding this comment.
함수명은 동사로 작성해주는게 좋아요. 또한 명확하게 어떤 값을 체크하는지 나타나면 좋습니다. 예를들어 validatePassword 이런식으로 쓰면 비밀번호는 체크하는 함수로 적절할 것 같아요.
There was a problem hiding this comment.
추후에는 components 라는 디텍토리를 오히려 UI 컴포넌트로 많이 사용하게될텐데요, 이러한 부분들은 함수를 저장하는 부분들이기 때문에 utils 등의 디렉토리로 가져가셔도 더 좋지않을까 생각이 듭니다.
There was a problem hiding this comment.
변수명 부분은 코드를 작성하는 주체인 저조차도 헷갈릴 정도로 작명이 미흡 했던것 같습니다. - 함수는 되도록 동사로 작성한다! -
파일 구성 방식은 아직 어떤 식으로 구성 하는게 좋을지 감이 잘 안잡히네요. 조금 더 노력이 필요할 것 같습니다. ㅠ
There was a problem hiding this comment.
넵 나중에 파일이 많아질 것을 대비하여 '목적에 맞게 분리한다'정도로 우선 가져가셔도 좋을 것 같아요. 추후에 React 등 프론트엔드 라이브러리 혹은 프레임워크를 사용하시다보면 자연스럽게 학습하게되시는 부분도 있으니 지금 너무 고민 안하셔도 될 것 같긴합니다 :)
| if (window.scrollY > 30) { | ||
| $nav.classList.add('nav_shadow'); | ||
| } else if (window.scrollY < 30) { | ||
| $nav.classList.remove('nav_shadow'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
조금 더 깔끔하게 작성한다면
if (window.scrollY > 30) {
$nav.classList.add('nav_shadow');
return;
}
$nav.classList.remove('nav_shadow');
이렇게도 쓸 수 있을 것 같아요. else if가 아닌 그냥 else를 써도 될 것 같고요!
There was a problem hiding this comment.
가독성이 확실히 좋아지는 것 같습니다.
헌데 if문 안에 return은 어떤 이유로 작성 하신 건지 궁금합니다!
There was a problem hiding this comment.
네 return을 쓴 이유는 보시면 아래 else가 없이 작성이 되어있어요. 즉 함수가 그 위에서 종료되고 아래쪽 .remove('nav_shadow)가 호출되지 않도록 하기위해서 return을 사용하였습니다. else를 쓰거나 위와같이 쓰거나 실제 동작은 동일합니다!
| --primary-color: #6d6afe; | ||
| --red-color: #ff5b56; | ||
| --black-color: #111322; | ||
| --white-color: #ffffff; | ||
| --gray1-color: #3e3e43; | ||
| --gray2-color: #9fa6b2; | ||
| --gray3-color: #ccd5e3; | ||
| --gray4-color: #e7effb; | ||
| --gray5-color: #f0f6ff; |
There was a problem hiding this comment.
이렇게 컬러를 정의한 파일은 따로 css파일로 묶으면 어떨까요?
공통적으로 다양한 페이지에서 사용하는 만큼 root에 color.css 를 정의하고
이를 다양한 페이지에서 태그를 통해 참조해서 사용해도 좋을 것 같습니다.
There was a problem hiding this comment.
css 파일도 잘 정리해서 작성해 보겠습니다!
| </div> | ||
| </footer> | ||
| </main> | ||
| <script type="module" src="../main.js"></script> |
There was a problem hiding this comment.
main.js가 커버하는 영역이 많은 것 같아요. 예를 들어 signUp button 이벤트 발생에 대한 처리를 해주는데, signin페이지에서는 해당 부분을 굳이 listening할 필요가 없으니까요.
js 파일도 페이지별로 쪼개서 관리하는게 어떨까요~?
There was a problem hiding this comment.
참고해서 한번 파일 관리를 어떻게 해야할지 고심해봐야 겠습니다!
| .hidden { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
이런 부분은도 마찬가지로 추후 동일하게 사용되는 부분이 많을 것이라고 생각이들어요(지금은 하나지만요!). 위에 color.css를 정의해서 반복해서 사용하는 것 처럼 이 부분도 따로 분리해서 관리한다면
많은 css 파일에서 이 부분을 반복적으로 작성할 필요없이 link tag를 통해 가져와서 쓰기만 하는 것으로 해결될 것 같습니다!
앞으로 반복될 여지가 많은 것들은 어떻게 분리해서 관리할까 생각해보셔도 좋을 것 같습니다 :)
요구사항
기본
심화
주요 변경사항
스크린샷
반응형 구현
비밀번호 표시 토글 & 회원가입 페이지에서 비밀번호 불일치시 메시지 출력 구현
멘토에게